🧹 Nitpick | 🔵 Trivial

Default data directory is evaluated at module load time.

getDefaultDataDir() is called once when the module loads, capturing process.cwd() at that moment. If the working directory changes before buildConfigFromEnvironment is called, the default will be stale.

Consider using a getter function for lazy evaluation:

 const ENSRainbowConfigSchema = z
   .object({
     port: PortSchema.default(ENSRAINBOW_DEFAULT_PORT),
-    dataDir: DataDirSchema.default(getDefaultDataDir()),
+    dataDir: DataDirSchema.optional(),
     dbSchemaVersion: DbSchemaVersionSchema,
     labelSet: LabelSetSchema.optional(),
   })

return ENSRainbowConfigSchema.parse({
  port: env.PORT,
  dataDir: env.DATA_DIR ?? getDefaultDataDir(),
  // ...
});

In `@apps/ensrainbow/src/config/config.schema.ts` around lines 33 - 36, The schema
currently calls getDefaultDataDir() at module load in ENSRainbowConfigSchema
(dataDir: DataDirSchema.default(getDefaultDataDir())), capturing process.cwd()
too early; remove the eager default from ENSRainbowConfigSchema and instead
handle lazy evaluation in buildConfigFromEnvironment by supplying dataDir:
env.DATA_DIR ?? getDefaultDataDir() when parsing/building the config, keeping
ENSRainbowConfigSchema (and DataDirSchema/PortSchema) purely declarative and
ensuring getDefaultDataDir() runs only at build time.

@djstrong Suggest reviewing this feedback from coderabbit

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

⚠️ Potential issue | 🟠 Major

process.exit(1) prevents testability and graceful error handling.

Calling process.exit(1) terminates the process immediately, making this function difficult to test and preventing callers from handling errors gracefully. Consider throwing a custom error and letting the caller decide how to handle it.

+export class ConfigurationError extends Error {
+  constructor(message: string) {
+    super(message);
+    this.name = "ConfigurationError";
+  }
+}
+
 export function buildConfigFromEnvironment(env: ENSRainbowEnvironment): ENSRainbowConfig {
   try {
     return ENSRainbowConfigSchema.parse({
       // ... parsing logic
     });
   } catch (error) {
     if (error instanceof ZodError) {
       logger.error(`Failed to parse environment configuration: \n${prettifyError(error)}\n`);
+      throw new ConfigurationError(`Invalid configuration: ${prettifyError(error)}`);
     } else if (error instanceof Error) {
       logger.error(error, `Failed to build ENSRainbowConfig`);
+      throw error;
     } else {
       logger.error(`Unknown Error`);
+      throw new ConfigurationError("Unknown configuration error");
     }
-
-    process.exit(1);
   }
 }

try {
  const config = buildConfigFromEnvironment(process.env as ENSRainbowEnvironment);
} catch (error) {
  process.exit(1);
}

In `@apps/ensrainbow/src/config/config.schema.ts` around lines 73 - 83, Replace
the terminal process.exit(1) in the catch block with throwing a descriptive
error so callers can handle failures; specifically, inside the catch for
buildConfigFromEnvironment (or whatever function constructs ENSRainbowConfig)
throw a custom error (e.g., ConfigBuildError) or rethrow the existing Error with
context including the prettified ZodError output and the message "Failed to
build ENSRainbowConfig", while keeping the existing logger calls for ZodError
and generic Error; move any process.exit(1) behavior out to the CLI/entrypoint
so tests can catch the thrown error and decide whether to exit.

🧹 Nitpick | 🔵 Trivial

Unsafe type assertion may mask configuration errors.

Casting process.env as ENSRainbowEnvironment bypasses TypeScript's type checking. While buildConfigFromEnvironment validates at runtime, the assertion could hide type mismatches during development.

Additionally, this function rebuilds the entire configuration on each call. If called frequently, consider memoizing or caching the result.

+let cachedConfig: ENSRainbowConfig | null = null;
+
 /**
  * Gets the port from environment variables.
  */
 export function getEnvPort(): number {
-  const config = buildConfigFromEnvironment(process.env as ENSRainbowEnvironment);
-  return config.port;
+  if (!cachedConfig) {
+    cachedConfig = buildConfigFromEnvironment(process.env as ENSRainbowEnvironment);
+  }
+  return cachedConfig.port;
 }

import type { ENSRainbowConfig } from "@/config/config.schema";

In `@apps/ensrainbow/src/lib/env.ts` around lines 7 - 10, The getEnvPort function
unsafely asserts process.env as ENSRainbowEnvironment and rebuilds the full
config on every call; remove the type assertion and instead import the
ENSRainbowConfig type (import type { ENSRainbowConfig } ...) and let
buildConfigFromEnvironment validate process.env at runtime, receiving an
ENSRainbowConfig result; then read and return config.port. Also memoize the
built config in a module-level variable so getEnvPort calls reuse the same
config instead of reconstructing it each time (references: getEnvPort,
buildConfigFromEnvironment, ENSRainbowEnvironment, ENSRainbowConfig).

This .check() invariant throws an Error directly. In most config invariants in this repo, the check adds a custom issue to ctx.issues so the failure is reported via ZodError and formatted by prettifyError (see apps/ensapi/src/config/validations.ts:18-24). Consider pushing a custom issue (with path: ["dbSchemaVersion"]) instead of throwing, so users get consistent, nicely formatted config errors.

The refactored getEnvPort() function now calls process.exit(1) on validation failures instead of throwing an error, which breaks error handling in code paths that expect a catchable error. This causes the application to exit unexpectedly when validatePortConfiguration() is called with an invalid PORT environment variable.

diff --git a/apps/ensrainbow/src/cli.test.ts b/apps/ensrainbow/src/cli.test.ts
index a8d674a4..b0159c00 100644
--- a/apps/ensrainbow/src/cli.test.ts
+++ b/apps/ensrainbow/src/cli.test.ts
@@ -51,20 +51,12 @@ describe("CLI", () => {
 
     it("should throw error for invalid port number", () => {
       process.env.PORT = "invalid";
-      const exitSpy = vi.spyOn(process, "exit").mockImplementation((() => {
-        throw new Error("process.exit called");
-      }) as never);
       expect(() => getEnvPort()).toThrow();
-      expect(exitSpy).toHaveBeenCalledWith(1);
     });
 
     it("should throw error for negative port number", () => {
       process.env.PORT = "-1";
-      const exitSpy = vi.spyOn(process, "exit").mockImplementation((() => {
-        throw new Error("process.exit called");
-      }) as never);
       expect(() => getEnvPort()).toThrow();
-      expect(exitSpy).toHaveBeenCalledWith(1);
     });
   });
 
diff --git a/apps/ensrainbow/src/config/config.schema.ts b/apps/ensrainbow/src/config/config.schema.ts
index e74f166c..6e079dc5 100644
--- a/apps/ensrainbow/src/config/config.schema.ts
+++ b/apps/ensrainbow/src/config/config.schema.ts
@@ -71,14 +71,15 @@ export function buildConfigFromEnvironment(env: ENSRainbowEnvironment): ENSRainb
           : undefined,
     });
   } catch (error) {
+    let errorMessage = `Failed to parse environment configuration`;
+
     if (error instanceof ZodError) {
-      logger.error(`Failed to parse environment configuration: \n${prettifyError(error)}\n`);
+      errorMessage = `Failed to parse environment configuration: \n${prettifyError(error)}\n`;
     } else if (error instanceof Error) {
-      logger.error(error, `Failed to build ENSRainbowConfig`);
-    } else {
-      logger.error(`Unknown Error`);
+      errorMessage = error.message;
     }
 
-    process.exit(1);
+    logger.error(errorMessage);
+    throw new Error(errorMessage);
   }
 }

PORT=invalid node cli.ts serve --port 5000

The config parameter is not used in the function body. While this might be intentional (similar to how ENSApi's buildEnsApiPublicConfig uses config), if there are no plans to use it, consider removing it. If it's kept for future extensibility or API consistency with other services, consider adding a comment explaining why it's present but unused, or using a TypeScript underscore prefix convention (e.g., _config) to indicate it's intentionally unused.

The zod version specified here (3.25.76) doesn't match the catalog version (^4.3.6) or what other apps in the monorepo use. The code imports from "zod/v4" which is specific to zod v4. This will cause import errors since zod v3 doesn't support the "/v4" import path. The specifier should remain as 'catalog:' to use zod 4.3.6 as defined in pnpm-workspace.yaml.

The config parameter is passed to this function but never used. The function only uses labelSet and recordsCount from the other parameters. Consider removing the unused parameter or documenting why it's kept for future use.

The JSDoc comment references a parameter "config" that doesn't exist in the function signature. The function takes "labelSet" and "recordsCount" as parameters, not a config object. The @param tag for "config" should be removed.

This is valid feedback. Please see my other related comment for how the private config really should be passed into this function.

The JSDoc comment for buildENSRainbowPublicConfig contains an incorrect parameter description that doesn't match the actual function signature.

# View the JSDoc and function signature:
cat apps/ensrainbow/src/config/config.schema.ts | sed -n '126,136p'

This is valid feedback. Please see my other related comment for how the private config really should be passed into this function.

Can you please create a follow-up issue on GitHub to remind us to take this action? Thanks

Suggest we remove these before merging. Can you verify it works locally as expected?

We have logger.debug in every endpoint should we remove all of them?

Prefer that we not use this strategy for implicitly defining what an ENSRainbowConfig is. This makes it more difficult for devs to figure out what this very important object is. It also means we don't have nice JSDocs on each of the fields on the config object.

Instead, it will improve our DX moving forward if you explicitly define what an ENSRainbowConfig is, similar to what we did here for ENSIndexerConfig: https://github.com/namehash/ensnode/blob/main/apps/ensindexer/src/config/types.ts

Please see my related comments on this.

The config object should reflect defaults and not pass on the complexity across our code that a key config value might be undefined.

This also should never be allowed to be undefined. The config object needs to trap certain complexity within itself, such as applying defaults. These complexities should not leak out to other layers of our code.

it looks like this should be converted into an invariant check such as these: https://github.com/namehash/ensnode/blob/main/apps/ensindexer/src/config/validations.ts

For the case of the serve command, what do you think about logging to the console the config that ENSRainbow is running with? Similar to https://github.com/namehash/ensnode/blob/main/apps/ensindexer/ponder/ponder.config.ts#L8-L13 except for ENSRainbow I assume there's no need to "redact" the config.